Hybrid shuffle #1387
Hybrid shuffle #1387
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1387 +/- ##
==========================================
+ Coverage 93.70% 93.73% +0.03%
==========================================
Files 223 224 +1
Lines 37561 37672 +111
==========================================
+ Hits 35195 35313 +118
+ Misses 2366 2359 -7 ☔ View full report in Codecov by Sentry. |
|
|
||
| /// Converts `IndistinguishableHybridReport` into | ||
| /// an `AdditiveShare` needed for shuffle protocol. | ||
| pub fn hybrid_report_to_shuffle_input<YS, BK, V>( |
There was a problem hiding this comment.
would it make more sense to implement these as From traits.
Right now, ctx.shuffle has the trait bound
I: IntoIterator<Item = AdditiveShare<S>> + Send,
but you could also consider updating that so that Item implements conversion into AdditiveShare<S>, rather than being an AdditiveShare directly.
Updating this now will either mean updating existing IPA to work the same way (yuck!), so maybe we wait for now.
| .into_iter() | ||
| .map(|item| shuffled_to_hybrid_report(&item)) |
There was a problem hiding this comment.
Nit: iter().map(shuffled_to_hybrid_report)
| .into_iter() | ||
| .map(|item| hybrid_report_to_shuffle_input::<BA112, BK, V>(&item)) |
There was a problem hiding this comment.
Nit: Don't think you need the owning iterator. See comment bellow.
|
|
||
| /// Converts an `AdditiveShare` obtained from shuffle protocol | ||
| /// into an `IndistinguishableHybridReport`. | ||
| pub fn shuffled_to_hybrid_report<YS, BK, V>( |
There was a problem hiding this comment.
these go away with the new sharded shuffle and its Shufflable trait?
| let world = TestWorld::default(); | ||
|
|
||
| let mut rng = rand::thread_rng(); | ||
| let mut records = Vec::new(); |
There was a problem hiding this comment.
nit: Can you do something like?
let records = (0..BATCHSIZE).map(|_| TestIndistinguishableHybridReport{ ... })
|
I don't think we should use the old shuffle as it is not sharded and it won't be possible to make it sharded. Instead, we should take the sharded implementation for Hybrid (protocol/prf/shuffle/sharded.rs) |
|
mmm, I didn't mean to close it, it was a misclick |
This implements the plumbing required to shuffle
IndistinguishableHybridReportsand adds the shuffle step to the protocol.As we discussed earlier, we may need to add the right Sharded + Malicious trait which will then make the
ctxused here have the sharded + malicious shuffle.